Skip to content

Conversation

zeripath
Copy link
Contributor

@zeripath zeripath commented Jul 1, 2020

Backport #12102

this was thought to be due to timeouts, however on closer look this
appears to be due to the Close() function of the BlameReader hanging
with a blocked stdout pipe.

This PR fixes this Close function to:

  • Cancel the context of the cmd
  • Close the StdoutReader - ensuring that the output pipe is closed

Further it makes the context of the git blame command a child of the
request context - ensuring that even if Close() is not called, on
cancellation of the Request the blame is command will also be cancelled.

Fixes #11716
Closes #11727

Signed-off-by: Andrew Thornton [email protected]

Backport go-gitea#12102

this was thought to be due to timeouts, however on closer look this
appears to be due to the Close() function of the BlameReader hanging
with a blocked stdout pipe.

This PR fixes this Close function to:

* Cancel the context of the cmd
* Close the StdoutReader - ensuring that the output pipe is closed

Further it makes the context of the `git blame` command a child of the
request context - ensuring that even if Close() is not called, on
cancellation of the Request the blame is command will also be cancelled.

Fixes go-gitea#11716
Closes go-gitea#11727

Signed-off-by: Andrew Thornton <[email protected]>
@zeripath zeripath added this to the 1.12.2 milestone Jul 1, 2020
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Jul 1, 2020
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jul 1, 2020
@lafriks lafriks merged commit 20c2bdf into go-gitea:release/v1.12 Jul 1, 2020
@zeripath zeripath deleted the backport-12102 branch July 1, 2020 16:01
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants